[alert,fpv] Remove nonsense AlertPingIgnored_A assertion#29647
Open
rswarbrick wants to merge 1 commit intolowRISC:masterfrom
Open
[alert,fpv] Remove nonsense AlertPingIgnored_A assertion#29647rswarbrick wants to merge 1 commit intolowRISC:masterfrom
rswarbrick wants to merge 1 commit intolowRISC:masterfrom
Conversation
The AlertPingIgnored_A assertion in the code before this commit didn't make all that much sense. It says: "If the alert sender is in the middle of replying to a ping and the alert receiver gets asked to send a new ping then the receiver's ping_ok_o output will be false until (and including) the first time that the receiver is being asked to send a ping" That's a rather convoluted way of saying: "If the alert sender is in the middle of replying to a ping and the alert receiver gets asked to send a new ping then the receiver's ping_ok_o output will be false". Since requests to the alert receiver are under the control of the environment in the formal testbench, *this* can be simplified to: "If the alert sender is in the middle of replying to a ping then the receiver's ping_ok_o output will be false". That is true, and it means that the receiver won't ever manufacture extra ping_ok_o signals. But doing so isn't really a concern: they would cause the spurious_alert_ping signal in alert_handler_ping_timer to go high, which is treated as a ping failure. I'm not entirely sure what the author intended from the assertion (added in c8fb0e6), but I can't really see any benefit to the code that's here. Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
AlertPingIgnored_Aassertion in the code before this commit didn't make all that much sense.It says:
That's a rather convoluted way of saying:
Since requests to the alert receiver are under the control of the environment in the formal testbench, this can be simplified to:
That is true, and it means that the receiver won't ever manufacture extra
ping_ok_osignals. But doing so isn't really a concern: they would cause thespurious_alert_pingsignal inalert_handler_ping_timerto go high, which is treated as a ping failure.I'm not entirely sure what the author intended from the assertion (added in c8fb0e6), but I can't really see any benefit to the code that's here.